Skip to content

Conversation

@b14ckyy
Copy link
Collaborator

@b14ckyy b14ckyy commented Dec 19, 2024

Removed some Preset values for Fixed wings due to FW changes that will be good on defaults.
Comes along with iNavFlight/inav#10541

@b14ckyy b14ckyy added this to the 8.0 milestone Dec 19, 2024
@mmosca
Copy link
Collaborator

mmosca commented Dec 19, 2024

Maybe instead of removing these, just adjust the airplane without a tail values as bredoven recommendations? We can look into removing this later, with more time for testing.

@b14ckyy
Copy link
Collaborator Author

b14ckyy commented Dec 19, 2024

If the controller now controls velocity and not position, I think the difference in wing load and pitch authority should not make a difference between them anymore. What is breadoven's recommendation? Could not see that in the 2 threads I follow. If they still act differently then yes I can add them back in with slightly different values.

@breadoven
Copy link
Collaborator

If the controller now controls velocity and not position, I think the difference in wing load and pitch authority should not make a difference between them anymore. What is breadoven's recommendation? Could not see that in the 2 threads I follow. If they still act differently then yes I can add them back in with slightly different values.

I don't see any reason why the velocity altitude control would affect the xy position control so these values should be left unchanged.

@b14ckyy
Copy link
Collaborator Author

b14ckyy commented Dec 19, 2024

@breadoven I removed the xy presets in that go as well since I noticed that somewhere in the 7.0 or 7.1 phase the value for flying wings with no tail was too low. I planned to change/remove them for a while and just remembered so I removed them now.
Especially I noticed with Path tracking and Autoland, that Wings struggle to get a precise heading fast enough on 55 while the Tail Planes worked much better. 75 or more on wings was severely better in tracking.

@b14ckyy
Copy link
Collaborator Author

b14ckyy commented Jan 18, 2025

I have restored the settings and just changed the defaults slightly based on the recommendations.

@mmosca mmosca modified the milestones: 8.0, 8.1 Jan 21, 2025
@mmosca
Copy link
Collaborator

mmosca commented Jan 21, 2025

This missed the train for 8.0. What is the concensus? Is it still needed?

@b14ckyy
Copy link
Collaborator Author

b14ckyy commented Jan 21, 2025

Not mandatory

@MrD-RC MrD-RC modified the milestones: 8.1, 9.0 Oct 26, 2025
@sensei-hacker sensei-hacker merged commit c16b5c8 into iNavFlight:master Oct 31, 2025
6 checks passed
@qodo-merge-pro
Copy link

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
No logging context: The added changes adjust default parameter values without introducing or modifying any
logging of critical actions, making it unclear whether related updates are audited.

Referred Code
            {
    key: "nav_fw_pos_z_p",
    value: 35
},
{
    key: "nav_fw_pos_z_i",
    value: 5
},
{
    key: "nav_fw_pos_z_d",
    value: 10
},
{
    key: "nav_fw_pos_xy_p",
    value: 55
},
{
    key: "fw_turn_assist_pitch_gain",
    value: 0.4
},
{


 ... (clipped 203 lines)
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
No error handling: The changes only modify static default values without any validation or error handling
additions, so it is unclear whether edge cases for these parameters are validated
elsewhere.

Referred Code
            {
    key: "nav_fw_pos_z_p",
    value: 35
},
{
    key: "nav_fw_pos_z_i",
    value: 5
},
{
    key: "nav_fw_pos_z_d",
    value: 10
},
{
    key: "nav_fw_pos_xy_p",
    value: 55
},
{
    key: "fw_turn_assist_pitch_gain",
    value: 0.4
},
{


 ... (clipped 203 lines)
Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Validation unknown: Updating default numeric parameters does not include input validation or constraints here,
leaving uncertainty about safeguards against invalid or unsafe values elsewhere.

Referred Code
            {
    key: "nav_fw_pos_z_p",
    value: 35
},
{
    key: "nav_fw_pos_z_i",
    value: 5
},
{
    key: "nav_fw_pos_z_d",
    value: 10
},
{
    key: "nav_fw_pos_xy_p",
    value: 55
},
{
    key: "fw_turn_assist_pitch_gain",
    value: 0.4
},
{


 ... (clipped 203 lines)
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants